-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
move to Cibuildwheel workflow #274
move to Cibuildwheel workflow #274
Conversation
The 1st run was really slow, over 3 hours for linux on arm builds. Those run on QEMU (emulator). I've disabled testing on all but the python 3.12 image. The bottleneck seems to be compiling. CCache wasn't active for the 1st run by design, lets see if the 2nd is faster. Unfortunately it only helps for musllinux as manylinux images miss ccache support. It is possible to speed up CI by splitting the workflow into more tasks, e.g. per python version or maybe The other problem with this PR: The mac on arm binaries don't work yet, it seems that the C++ part needs to respect cross compilation, similar to scikit which fixed this problem in scikit-build/scikit-build#530, scikit-build/scikit-build#555, scikit-build/scikit-build#556 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just had a few suggestions for setup.py.
Btw, it seems you are trying to enforce 80 chars line limit ?
I guess it is somewhat short nowadays, but if you think we should have it, then maybe better we have something like .editorconfig
?
Also seems we have lot of other code, that doesn't comply with it.
fwiw, I had no chance to work with Cibuildwheel
before, but am glad we are using something more modern, that should make our life better.
@narekgharibyan No intention to enforce a character limit. I just switched to a new box, used vscode and let it format the file. The limit must be the default as we are missing a configuration for all of the python code. I happily add a config and reformat accordingly. Do you have a config that you consider modern/standard these days? |
What about adding a |
Yup sounds good ... fwiw, I didn't play with |
start with ubuntu first
skip musllinux
add macos
07d42ce
to
16f7a92
Compare
I reformatted setup.py with ruff. Unfortunately I had to revert my checkin of I think we need a renovation of the python code base according to the newest standards, however not as part of this PR. Created #275 |
switch the build system for python to cibuildwheel. cibuildwheel makes it easier to build python wheels on different platforms and python versions. The switch hopefully makes our life a little easier and lets us concentrate on the more important things.
With the switch we get:
In addition to the change I implemented the following additions in this PR:
superseds #269